[bgp/agg]: Add BGP aggregate address test cases for Config Persistence and Recovery by shixizhang · Pull Request #23347 · sonic-net/sonic-mgmt

shixizhang · 2026-03-26T11:46:02Z

Description of PR

Summary:
Add new test file test_bgp_aggregate_address_resilience.py (Test Group 5) that validates BGP aggregate-address configuration persistence and recovery across various disruption scenarios. These 5 new test cases verify that aggregate address configuration written via GCU survives BGP container restarts, config reloads, cold reboots, warm reboots, and BBR state transitions.

New test cases:

TC 5.1 test_aggregate_persists_bgp_container_restart: Aggregate config survives BGP container restart; CONFIG_DB + STATE_DB + FRR are consistent after recovery.
TC 5.2 test_aggregate_persists_config_reload: Aggregate config (with summary-only=true) survives config save + config reload.
TC 5.3 test_aggregate_persists_config_save_and_reboot: IPv6 aggregate config survives config save + cold reboot.
TC 5.4 test_aggregate_bbr_required_inactive_persists_bgp_restart: BBR-required aggregate stays inactive after BGP restart when BBR is disabled; activates once BBR is enabled.
TC 5.5 test_aggregate_persists_warm_reboot: Aggregate config survives warm reboot.

Type of change

Back port request

Approach

What is the motivation for this PR?

Existing BGP aggregate-address tests cover configuration validation and route propagation behavior, but there are no tests verifying that aggregate address configuration persists across operational disruptions such as BGP container restarts, config reloads, and device reboots. This PR fills that gap by adding resilience tests that validate CONFIG_DB, STATE_DB, and FRR running-config consistency after each disruption type.

How did you do it?

Created test_bgp_aggregate_address_resilience.py reusing existing helpers and fixtures from test_bgp_aggregate_address.py (AggregateCfg, gcu_add_aggregate, gcu_remove_aggregate, verify_bgp_aggregate_consistence, verify_bgp_aggregate_cleanup, dump_db, and the setup_teardown checkpoint/rollback fixture).
Added a bgp_neighbors fixture to discover BGP neighbor IPs for session-state polling after disruptions.
Pre-disruption verification only checks CONFIG_DB (GCU write is synchronous). Post-disruption verification checks the full stack (CONFIG_DB + STATE_DB + FRR) after bgpcfgd has re-processed the config.
Added wait_for_aggregate_state() helper to handle the asynchronous bgpcfgd STATE_DB population after disruptions.
All test cases include proper cleanup in finally blocks with graceful fallback to checkpoint rollback.

How did you verify/test it?

Ran all test cases on a physical m1-48 testbed with Arista EOS neighbors.

Any platform specific information?

No platform-specific dependencies. Tests use GCU for configuration and standard SONiC reboot/reload utilities, which are platform-agnostic.

Supported testbed topology if it's a new test case?

t1, m1 (declared via @pytest.mark.topology("t1", "m1"))

Documentation

Aligned with BGP-Aggregate-Address test plan

mssonicbld · 2026-03-26T11:46:09Z

/azp run

azure-pipelines · 2026-03-26T11:46:24Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-03-27T02:04:02Z

/azp run

azure-pipelines · 2026-03-27T02:04:14Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-03-27T05:30:20Z

/azp run

azure-pipelines · 2026-03-27T05:30:33Z

Azure Pipelines successfully started running 1 pipeline(s).

Co-authored-by: Copilot <[email protected]> Signed-off-by: Shixi Zhang <[email protected]>

mssonicbld · 2026-03-27T05:37:57Z

/azp run

azure-pipelines · 2026-03-27T05:38:10Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-03-29T03:20:27Z

/azp run

azure-pipelines · 2026-03-29T03:20:41Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-03-30T02:56:23Z

/azp run

azure-pipelines · 2026-03-30T02:56:37Z

Azure Pipelines successfully started running 1 pipeline(s).

… patch On KVM/VS the warm-reboot script has a 1s timeout for docker-exec health checks that is too tight, causing fpmsyncd and orchagent to crash during warm reboot. This is the same issue that AdvancedReboot handles by patching the timeout to 5s (tests/common/fixtures/advanced_reboot.py). Apply the same sed patch to the warm-reboot script before rebooting, and restore the original safe_reboot=True call with full recovery checks. Co-authored-by: Copilot <[email protected]> Signed-off-by: Shixi Zhang <[email protected]>

mssonicbld · 2026-03-30T05:57:23Z

/azp run

azure-pipelines · 2026-03-30T05:57:38Z

Azure Pipelines successfully started running 1 pipeline(s).

StormLiangMS

Hi @shixizhang, nice test cases! One issue:

\pytest.mark.device_type('vs')\ contradicts physical test verification - The PR description says tests were validated on a physical m1-48 testbed, but \device_type('vs')\ restricts execution to VS devices when \--device_type\ is specified. Please use \pytest.mark.device_type('physical')\ or remove the marker entirely.

Also a minor concern: TC5.2/TC5.3 do \sudo config save -y\ before cleanup. If the test fails after save but before teardown, the aggregate entry persists across reboots. Please ensure the \setup_teardown\ fixture handles rollback of saved config.

mssonicbld · 2026-03-30T10:16:06Z

/azp run

azure-pipelines · 2026-03-30T10:16:20Z

Azure Pipelines successfully started running 1 pipeline(s).

- Remove device_type('vs') marker that contradicts physical testbed validation - Add local setup_teardown fixture with config save after rollback to prevent aggregate entries from persisting in on-disk config across reboots Co-Authored-By: Claude Opus 4.6 <[email protected]>

mssonicbld · 2026-03-31T02:45:38Z

/azp run

linux-foundation-easycla · 2026-03-31T02:45:40Z

The committers listed above are authorized under a signed CLA.

✅ login: Copilot / name: Copilot (2babda5, 391c099, 48ec752, a343dab)
✅ login: shixizhang / name: Shixi Zhang (2babda5, 391c099, 48ec752, a343dab)

azure-pipelines · 2026-03-31T02:45:50Z

Azure Pipelines successfully started running 1 pipeline(s).

shixizhang · 2026-03-31T02:52:21Z

Hi @shixizhang, nice test cases! One issue:

\pytest.mark.device_type('vs')\ contradicts physical test verification - The PR description says tests were validated on a physical m1-48 testbed, but \device_type('vs')\ restricts execution to VS devices when --device_type\ is specified. Please use \pytest.mark.device_type('physical')\ or remove the marker entirely.

Also a minor concern: TC5.2/TC5.3 do \sudo config save -y\ before cleanup. If the test fails after save but before teardown, the aggregate entry persists across reboots. Please ensure the \setup_teardown\ fixture handles rollback of saved config.

addressed the issue as new iteration.

- Remove device_type('vs') marker that contradicts physical testbed validation - Add local setup_teardown fixture with config save after rollback to prevent aggregate entries from persisting in on-disk config across reboots Co-authored-by: Copilot <[email protected]>

mssonicbld · 2026-03-31T02:58:19Z

/azp run

azure-pipelines · 2026-03-31T02:58:32Z

Azure Pipelines successfully started running 1 pipeline(s).

- Remove device_type('vs') marker that contradicts physical testbed validation - Add local setup_teardown fixture with config save after rollback to prevent aggregate entries from persisting in on-disk config across reboots Co-authored-by: Copilot <[email protected]> Signed-off-by: Shixi Zhang <[email protected]>

mssonicbld · 2026-03-31T03:02:56Z

/azp run

azure-pipelines · 2026-03-31T03:03:10Z

Azure Pipelines successfully started running 1 pipeline(s).

- Remove device_type('vs') marker that contradicts physical testbed validation - Add local setup_teardown fixture with config save after rollback to prevent aggregate entries from persisting in on-disk config across reboots Co-authored-by: Copilot <[email protected]> Signed-off-by: Shixi Zhang <[email protected]>

mssonicbld · 2026-03-31T03:28:36Z

/azp run

azure-pipelines · 2026-03-31T03:28:49Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-03-31T07:31:38Z

/azp run

azure-pipelines · 2026-03-31T07:31:53Z

Azure Pipelines successfully started running 1 pipeline(s).

- Sync is_virtual_platform(duthost) fix for warm reboot KVM check - Change topology mark from (t1, m1) to m1 only Co-authored-by: Copilot <[email protected]> Signed-off-by: Shixi Zhang <[email protected]>

mssonicbld · 2026-03-31T07:35:21Z

/azp run

azure-pipelines · 2026-03-31T07:35:36Z

Azure Pipelines successfully started running 1 pipeline(s).

shixizhang · 2026-04-01T02:41:31Z

@StormLiangMS Thanks for the review! Both issues have been addressed:

device_type('vs') removed — Topology is now restricted to m1 only, no device_type marker.
Config save rollback — Added a dedicated setup_teardown fixture that creates a checkpoint before tests and on teardown runs rollback_or_reload + config save -y, so even if a test fails after save but before cleanup, the on-disk config is restored to a clean state.
Please take another look when you get a chance.

StormLiangMS

@shixizhang — Approved ✅

All prior review comments addressed in commits a343dab and 2babda5:

✅ \device_type('vs')\ removed — replaced with \is_virtual_platform(duthost)\ runtime check in TC 5.5 only (correct approach)
✅ *Topology narrowed to \m1* — appropriate for aggregate-address tests that need real multi-ASIC or physical topology
✅ \setup_teardown\ fixture now does \config save -y\ after rollback — prevents stale aggregate entries from persisting in on-disk config across reboots

Code quality observations:

Clean reuse of helpers from \ est_bgp_aggregate_address.py\ — no duplication
Good pre/post disruption verification strategy (CONFIG_DB only pre, full stack post)
\wait_for_aggregate_state()\ helper with polling handles the async bgpcfgd race correctly
All 5 test cases have proper try/finally cleanup with graceful fallback to checkpoint rollback
TC 5.4 (BBR) properly saves and restores BBR default state
Warm reboot VS timeout patch mirrors the established pattern from \�dvanced_reboot.py\

CI: All required checks passing. The only failure is the OPTIONAL t1-lag-vpp test — unrelated.

LGTM — ready to merge.

github-actions bot requested review from cyw233, lolyu and sanjair-git March 26, 2026 11:46

shixizhang enabled auto-merge (squash) March 26, 2026 11:47

shixizhang force-pushed the addbgptest-pr branch from f123c6f to e967b7f Compare March 27, 2026 02:03

Copilot AI mentioned this pull request Mar 27, 2026

Review all human-authored PRs opened in the past 24 hours in sonic-net/sonic-mgmt #23365

Draft

shixizhang force-pushed the addbgptest-pr branch from e967b7f to 27700c7 Compare March 27, 2026 05:30

Add BGP aggregate address resilience test

391c099

Co-authored-by: Copilot <[email protected]> Signed-off-by: Shixi Zhang <[email protected]>

shixizhang force-pushed the addbgptest-pr branch from 27700c7 to 391c099 Compare March 27, 2026 05:37

shixizhang force-pushed the addbgptest-pr branch from 489d455 to 759fc61 Compare March 30, 2026 02:56

shixizhang force-pushed the addbgptest-pr branch from 759fc61 to 48ec752 Compare March 30, 2026 05:57

StormLiangMS requested changes Mar 30, 2026

View reviewed changes

shixizhang closed this Mar 30, 2026

auto-merge was automatically disabled March 30, 2026 10:15
Pull request was closed

shixizhang reopened this Mar 30, 2026

shixizhang force-pushed the addbgptest-pr branch from 6665622 to 427f94c Compare March 31, 2026 02:58

shixizhang force-pushed the addbgptest-pr branch from 427f94c to 9d96cec Compare March 31, 2026 03:02

shixizhang force-pushed the addbgptest-pr branch from 9d96cec to a343dab Compare March 31, 2026 03:27

[bgp/agg]: Use is_virtual_platform() and restrict topology to m1

2babda5

- Sync is_virtual_platform(duthost) fix for warm reboot KVM check - Change topology mark from (t1, m1) to m1 only Co-authored-by: Copilot <[email protected]> Signed-off-by: Shixi Zhang <[email protected]>

shixizhang force-pushed the addbgptest-pr branch from 132d14e to 2babda5 Compare March 31, 2026 07:35

shixizhang requested a review from StormLiangMS March 31, 2026 09:35

shixizhang enabled auto-merge (squash) April 1, 2026 02:20

StormLiangMS approved these changes Apr 1, 2026

View reviewed changes

shixizhang merged commit 6f61a88 into sonic-net:master Apr 1, 2026
18 of 19 checks passed

Conversation

shixizhang commented Mar 26, 2026

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

mssonicbld commented Mar 26, 2026

Uh oh!

azure-pipelines bot commented Mar 26, 2026

Uh oh!

mssonicbld commented Mar 27, 2026

Uh oh!

azure-pipelines bot commented Mar 27, 2026

Uh oh!

mssonicbld commented Mar 27, 2026

Uh oh!

azure-pipelines bot commented Mar 27, 2026

Uh oh!

mssonicbld commented Mar 27, 2026

Uh oh!

azure-pipelines bot commented Mar 27, 2026

Uh oh!

mssonicbld commented Mar 29, 2026

Uh oh!

azure-pipelines bot commented Mar 29, 2026

Uh oh!

mssonicbld commented Mar 30, 2026

Uh oh!

azure-pipelines bot commented Mar 30, 2026

Uh oh!

mssonicbld commented Mar 30, 2026

Uh oh!

azure-pipelines bot commented Mar 30, 2026

Uh oh!

StormLiangMS left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Mar 30, 2026

Uh oh!

azure-pipelines bot commented Mar 30, 2026

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

linux-foundation-easycla bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines bot commented Mar 31, 2026

Uh oh!

shixizhang commented Mar 31, 2026

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

azure-pipelines bot commented Mar 31, 2026

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

azure-pipelines bot commented Mar 31, 2026

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

azure-pipelines bot commented Mar 31, 2026

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

azure-pipelines bot commented Mar 31, 2026

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

azure-pipelines bot commented Mar 31, 2026

Uh oh!

shixizhang commented Apr 1, 2026

Uh oh!

StormLiangMS left a comment

linux-foundation-easycla bot commented Mar 31, 2026 •

edited

Loading